Size Change: -36 B (0%)

Total Size: 1.77 MB

_{compressed-size-action}

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <[email protected]>
Co-authored-by: fabiankaegy <[email protected]>
Co-authored-by: stokesman <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@fabiankaegy Thanks for the feedback!

In my mind we should only remove the isGutenbergPlugin part from the check

Do you mean the following conditional statement?

hasV3BlocksOnly || isBlockBasedTheme || isEditingTemplate || isZoomOutMode;

I think such a change would have a very big impact when this PR ships to core.

For example, plugin developers may intentionally pin their blocks' API version to 2 because they're not yet ready to support the iframe editor. Adding the isBlockBasedTheme check may mean that their blocks won't work properly when the block theme is activated on sites where these plugins are used.

I think we need to ensure that the editor is not iframed if all blocks are not v3 for now. See the documentation here.

This PR aims to make the editor behavior consistent whether the Gutenberg plugin is enabled or not. It does not extend the coverage of the iframe editor.

Could such a suggestion be considered in a follow-up?

@t-hamano I'll defer to @ellatrix and @youknowriad on this :)

My understanding was always that block themes just always get the iframe. Regardless of the API version. Because the block are already broken in the site editor which currently only exists in an iframe. And we as core want to push for the iframe because it is the better solution.

I agree however that the messaging around V3 of the block API made this a lot more confusing.

Jetpack for example still uses API version 1 in a lot of their stuff because they never got around to update their blocks for the impactful V2 update. But all of their blocks support the iframed site editor just fine. You can see some of that convo here: Automattic/jetpack#34120

I've removed my request for changes as I agree what I'm suggesting is more of a change where as what is in this PR today is a fix :) So we can get this in and then argue about what it should be like going forward separately 👍

Thank you all for your reviews.

In WP6.7, the editor will work as an iframe even if meta boxes are present, which will have some impact on developers. Based on their feedback, we can reconsider whether always to enable iframes in block-based themes.

Hey folks, I'm not sure I understand or agree with this change.

This condition was there as a way to let users know that at some point in Core, all block themes will be iframed by default regardless of the other conditions. So this change feels backwards to me. Instead of moving towards more iframing, we're going back to less iframing.

@youknowriad Thanks for the feedback.

I agree that in the future block themes should iframe the editor by default. However, having the editor behave differently depending on whether the Gutenberg plugin is enabled seems confusing and hard to test.

Even taking this into account, should we revert this PR?

I understand, it's true that the plugin behaves differently than Core but it's kind of the same like all the features built on the plugin and that at some point make its way into Core. (Alpha). So I consider this check as "alpha" basically.

Personally, I would be in favor of reverting

I see, I'll submit a PR to revert this change.

In the future, we might be able to consider just removing the isGutenbergPlugin check, as @fabiankaegy suggested.

Just to note it seems like the reason to do this was to make testing more obvious and avoid shipping bugs #64247 (comment) and that’s not a bad thing. I can see the reason to keep it as is though. Perhaps there’s another condition we could add so that for Gutenberg development it acts like it’s not the plugin 🤔. Or we can just strive to test scrupulously.

I would like to raise a little bit of a discussion about the future direction. In the future, will the editor always default to an iframe, i.e. will the useShouldIframe hook be removed?

In that case, I am concerned that Block API version 3 and version 2 will essentially be the same.

I would like to raise a little bit of a discussion about the future direction. In the future, will the editor always default to an iframe, i.e. will the useShouldIframe hook be removed?

Yes, I think we should strive for that.

In that case, I am concerned that Block API version 3 and version 2 will essentially be the same.

Yes, I'm not sure that's an issue though, the version 3 was created for that specific reason, to give block authors some time to migrate and test the iframe. What do you have in mind here?

the version 3 was created for that specific reason, to give block authors some time to migrate and test the iframe. What do you have in mind here?

I just felt a little strange that the API versions were different but worked exactly the same 😅

Other than that, I have no concerns.